-
-
Notifications
You must be signed in to change notification settings - Fork 349
Add commit tests #768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add commit tests #768
Conversation
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
{craft}
extensions: curl,ast
sapi: cli,micro
{/craft}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces default handling for shared extensions, updates documentation to reflect the new option, and adds a GitHub Actions workflow to parse commit message tags and run craft/bash tests automatically.
- Provide an empty-array default for
shared-extensionsinCraftCommand - Document the
shared-extensionskey underbuild-options - Add
.github/workflows/commit-tests.ymlto conditionally run tests based on{craft}and{bash}tags
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/SPC/command/CraftCommand.php | Default missing shared-extensions to an empty array |
| docs/deps-craft-yml.md | Add shared-extensions: [] sample under build-options |
| .github/workflows/commit-tests.yml | New workflow to parse commit tags and run craft/bash tests |
Comments suppressed due to low confidence (6)
src/SPC/command/CraftCommand.php:52
- Add or update unit/integration tests to cover the scenario when
shared-extensionsis not provided, ensuring that the default empty array case behaves as expected.
$shared_extensions = implode(',', $craft['shared-extensions'] ?? []);
docs/deps-craft-yml.md:45
- Indent this comment line and the following key so they correctly appear under the
build-optionsblock with the same two-space indentation as the other YAML entries.
# Build options for shared extensions (same as `build-shared` command options, all options are optional)
.github/workflows/commit-tests.yml:35
- This
exit 0halts the entire parsing script when{craft}tags are missing, preventing downstream parsing of Bash tags or prefix/OS settings. Consider removing or replacing it to allow the script to continue processing.
exit 0
.github/workflows/commit-tests.yml:36
- Explicitly set
skip_craft=nowhen craft tags are found to make the workflow logic clearer and avoid relying on an empty default.
else
.github/workflows/commit-tests.yml:51
- In the Bash parsing block, explicitly set
skip_bash=nowhen{bash}tags are found to make the condition more transparent.
else
.github/workflows/commit-tests.yml:1
- [nitpick] The workflow
nameisSingle Testbut it runs both craft and bash jobs. Consider renaming it to something likeCommit Teststo better reflect its purpose.
name: Single Test
{craft}
extensions: bcmath
shared-extensions: xdebug,swoole
sapi: cli
{/craft}
[spc_prefix:bin/spc-gnu-docker]
2a1e1b6 to
0e88cdb
Compare
|
I'm on a short weekend trip, I can leave a proper review tomorrow evening |
Ah no problem. I'll leave tomorrow, too. Enjoy your weekend! |
|
Looks good to me now. I also updated the PR template. |
What does this PR do?
Checklist before merging
*.php, runcomposer cs-fixat local machine.src/global/test-extensions.php../docs/.config/xxx.jsoncontent, runbin/spc dev:sort-config xxx.